Skip to content

Conversation

@daveverwer
Copy link
Member

Supercedes #3474

Well, what a rabbit hole this turned out to be!

Thank you so much @leogdion for this contribution, and please don’t take the fact that I completely rewrote it as any slight on your code.

Once I realised what you were doing with removing and then re-populating the pre element with the Mermaid source, it didn’t seem like the best approach. I changed things around and now we use render on mermaid to grab SVG data directly and insert both a light and dark mode chart into a new container.

This has several advantages:

  1. No need to run JavaScript when switching between light and dark mode. We can do with CSS just like we do for the rest of the site.
  2. We get to style the div as we like. You mentioned on our call this afternoon that you wanted to try and centre the chart which I have now done. It also removes it from the unnecessary pre styling with a grey background. I think it looks better this way, and it gives more options for future styling.

The disadvantage is that we now need to render every chart twice. I can’t see that this would be a performance problem, but it’s something to watch out for.

Thank you again. This is a great step forward for README files on SPI!

@leogdion
Copy link
Contributor

Great job! @daveverwer
Is there a way I can see it in action?

@daveverwer
Copy link
Member Author

Is there a way I can see it in action?

The easiest way would be to run it locally, but here are some screenshots:

Screenshot 2024-11-15 at 09 06 55@2x

and in dark mode

Screenshot 2024-11-15 at 09 06 29@2x

@daveverwer daveverwer merged commit 92a024f into main Nov 15, 2024
6 checks passed
@daveverwer daveverwer deleted the 3348-mermaid-chart branch November 15, 2024 11:10
@daveverwer
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants